DB/RestrictedClasses: remove redundant call to parent::setUpPrerequisites()
#2578
Conversation
…sites()` Calling `AbstractSniffUnitTest::setUpPrerequisites()` manually inside `RestrictedClassesUnitTest::enhanceGroups()` is not necessary as this method already executes via its `@before` tag and results in the method running twice. Originally, `RestrictedClassesUnitTest::enhanceGroups()` was named `RestrictedClassesUnitTest::setUp()` and called `parent::setUp()` (https://github.com/WordPress/WordPress-Coding-Standards/blob/3b9ae92607295548df357e108fd27090cf89d1d7/WordPress/Tests/DB/RestrictedClassesUnitTest.php#L34-L35). But later it was renamed, and then it probably didn't make sense to continue calling `parent::setUp()` (which was later renamed to `setUpPrerequisites()`): WordPress@00b895c. I found this in the context of the work to prepare this codebase for PHPCS 4.0 as `AbstractSniffUnitTest::setUpPrerequisites()` was renamed in 4.0 (PHPCSStandards/PHP_CodeSniffer@92cf10f#diff-91ba2a2a923bbfdaa61dcd31051d783bd0e7ff99f979f6e1a2917cae7dde3eb2L58).
There was a problem hiding this comment.
Good find @rodrigoprimo and good explanation.
While this fix is valid as-is for the current codebase, I want to point out that this will need to be brought back again once support for PHPCS 3.x will be dropped.
Reasoning:
- Drop PHPCS 3.x means => drop support for PHP 7.2 means => no need to support PHPUnit 4-7 anymore.
- Once support for PHPUnit 4-7 has been dropped, we can remove the
@before/@afterannotations and start using the normalsetUp()/tearDown()methods again (as the required return typevoidis no longer a problem), in which case, we will need to callparent::setUp()again.
Not sure if you already have some commits in your WIP branch for the follow ups (add cross-version PHPCS 3/4 support => drop support for PHP < 7.2 => drop support for PHPUnit < 8 => drop support for PHPCS 3.x => add support for PHPUnit 10+11). If so, please make sure you add it back in the appropriate follow-up commit.
Otherwise, we should maybe make a note of this somewhere ?
|
Thanks for the review, @jrfnl!
Thanks for checking. Yes, I have a commit ready for this in a WIP branch. |
Calling
AbstractSniffUnitTest::setUpPrerequisites()manually insideRestrictedClassesUnitTest::enhanceGroups()is not necessary, as this method already executes via its@beforetag and results in the method running twice.Originally,
RestrictedClassesUnitTest::enhanceGroups()was namedRestrictedClassesUnitTest::setUp()and calledparent::setUp()(WordPress/Tests/DB/RestrictedClassesUnitTest.php#L34-L35). But later it was renamed, and then it probably didn't make sense to continue callingparent::setUp()(which was later renamed tosetUpPrerequisites()): 00b895c.I found this in the context of the work to prepare this codebase for PHPCS 4.0 as
AbstractSniffUnitTest::setUpPrerequisites()was renamed in 4.0 (PHPCSStandards/PHP_CodeSniffer@92cf10f#diff-91ba2a2a923bbfdaa61dcd31051d783bd0e7ff99f979f6e1a2917cae7dde3eb2L58).